Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Change theano imports to aesara #83

Merged
merged 4 commits into from
May 11, 2021
Merged

Change theano imports to aesara #83

merged 4 commits into from
May 11, 2021

Conversation

fanshi118
Copy link
Contributor

Thank you for opening a PR!

Here are a few important guidelines and requirements to check before your PR can be merged:

  • There is an informative high-level description of the changes.
  • The description and/or commit message(s) references the relevant GitHub issue(s).
  • The commit messages follow these guidelines.
  • The commits correspond to relevant logical changes.
  • There are tests covering the changes introduced in the PR.

Don't worry, your PR doesn't need to be in perfect order to submit it. As development progresses and/or reviewers request changes, you can always rewrite the history of your feature/PR branches.

If your PR is an ongoing effort and you would like to involve us in the process, simply make it a draft PR.

@fanshi118
Copy link
Contributor Author

I changed the imports from theano to aesara but left aliases like ts and tt as they are for the moment (will get back to editing those once the tests are working fine). Here is the testing result that I got so far >>

======================================================= short test summary info =======================================================
FAILED tests/test_distributions.py::test_DiscreteMarkovChain_str - TypeError: ufunc 'isfinite' not supported for the input types, an...
FAILED tests/test_distributions.py::test_DiscreteMarkovChain_random - ValueError: Unexpected type in draw_value: <class 'aesara.tens...
FAILED tests/test_distributions.py::test_DiscreteMarkovChain_point - NotImplementedError: Cannot convert gamma_0 ~ Dirichlet to a te...
FAILED tests/test_distributions.py::test_DiscreteMarkovChain_logp - aesara.link.c.exceptions.CompileError: Compilation failed (retur...
FAILED tests/test_distributions.py::test_SwitchingProcess_random - NotImplementedError: Cannot convert TensorConstant{0} to a tensor...
FAILED tests/test_distributions.py::test_PoissonZeroProcess_point - NotImplementedError: Cannot convert Elemwise{Cast{float64}}.0 to...
FAILED tests/test_distributions.py::test_random_PoissonZeroProcess_DiscreteMarkovChain - NotImplementedError: Cannot convert p_0 ~ D...
FAILED tests/test_distributions.py::test_SwitchingProcess - NotImplementedError: Cannot convert TensorConstant{0} to a tensor variable.
FAILED tests/test_distributions.py::test_subset_args - NotImplementedError: Cannot convert TensorConstant{[0.1 1.2 2.3]} to a tensor...
FAILED tests/test_estimation.py::test_only_positive_state - NotImplementedError: Cannot convert p_0 ~ Dirichlet to a tensor variable.
FAILED tests/test_estimation.py::test_time_varying_model - aesara.link.c.exceptions.CompileError: Compilation failed (return status=1):
FAILED tests/test_step_methods.py::test_FFBSStep - TypeError: ufunc 'isfinite' not supported for the input types, and the inputs cou...
FAILED tests/test_step_methods.py::test_FFBSStep_extreme - NotImplementedError: Cannot convert p_0 ~ Dirichlet to a tensor variable.
FAILED tests/test_step_methods.py::test_TransMatConjugateStep - NotImplementedError: Cannot convert p_0 ~ Dirichlet to a tensor vari...
FAILED tests/test_step_methods.py::test_TransMatConjugateStep_subtensors - NotImplementedError: Cannot convert p_0 ~ Dirichlet to a ...
FAILED tests/test_utils.py::test_logsumexp[test_input2] - aesara.link.c.exceptions.CompileError: Compilation failed (return status=1):
FAILED tests/test_utils.py::test_tt_logdotexp - aesara.link.c.exceptions.CompileError: Compilation failed (return status=1):
FAILED tests/test_utils.py::test_multilogit_inv[test_input0-test_output0] - aesara.link.c.exceptions.CompileError: Compilation faile...
FAILED tests/test_utils.py::test_multilogit_inv[test_input1-test_output1] - aesara.link.c.exceptions.CompileError: Compilation faile...
FAILED tests/test_utils.py::test_multilogit_inv[test_input2-test_output2] - aesara.link.c.exceptions.CompileError: Compilation faile...
FAILED tests/test_utils.py::test_multilogit_inv[test_input3-test_output3] - aesara.link.c.exceptions.CompileError: Compilation faile...
============================================= 21 failed, 5 passed, 47 warnings in 39.59s ==============================================

Looks like there are a few that happened in different tests. How should we go about fixing some of these?

brandonwillard
brandonwillard suggested changes May 6, 2021
View reviewed changes
Copy link
Contributor

@brandonwillard brandonwillard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a tricky situation, because the PyMC3 (version 3) dependency will always entail the use of Theano-PyMC, but we want to preempt the transition to Aesara that's coming with version 4.

Right now, all we can do is remove the explicit theano and aesara dependencies in setup.py and allow the pymc3 dependency to determine which one is used. For this to work, we need to change statements like import theano to

try:
    import theano as aesara
except ImportError:
    import aesara

To test that this works, we need to run the test suite with a standard PyMC3 release that uses Theano-PyMC (i.e. test the non-exception case) and also the current PyMC3 master branch (i.e. the exception case), which is a version 3 branch that uses Aesara.

tests/test_distributions.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
@fanshi118 fanshi118 force-pushed the aesara branch 2 times, most recently from 442b130 to 38ed07c Compare May 11, 2021 01:20
@brandonwillard brandonwillard merged commit ca7f869 into main May 11, 2021
@brandonwillard brandonwillard deleted the aesara branch May 11, 2021 22:15
@fanshi118 fanshi118 linked an issue May 12, 2021 that may be closed by this pull request
Use aesara instead of theano #69
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brandonwillard brandonwillard brandonwillard requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use aesara instead of theano
2 participants
@fanshi118 @brandonwillard